-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Don't crash on error codes passed to --explain
which exceed our internal limit of 9999
#140700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
fmease is on vacation. Please choose another assignee. |
r? @davidtwco rustbot has assigned @davidtwco. Use |
This comment has been minimized.
This comment has been minimized.
I feel like this is very arbitrary. The
to behave the same as
and
to gracefully fail with the same error message. |
@jieyouxu any actual idea why tidy start fails here?
like... should I just add E9999 there..? |
Some changes occurred in diagnostic error codes |
oh no, looks like adding E9999 was a miskate here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any actual idea why tidy start fails here?
I'm pretty sure it's simply due to tidy (src/tools/tidy/src/error_codes.rs:361
) grepping through ~all Rust compiler source files looking for \bE\d{4}\b
and finding the E9999
in your previous error message that you've since removed.
This comment has been minimized.
This comment has been minimized.
affc073
to
7af4bb6
Compare
You may want to add a regression test based on |
@ShE3py oh, yes i want to add tests for it, didnt knew that there is some for yeah, i guess i cant do this in one file |
Yup, that's unfortunate but AFAIK you can only run Btw, |
yep, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two suggestions then we should be good to go!
@fmease seems done, but check pls if i got you and made all right |
If you'd like to, go right ahead, sure sounds like a good idea. |
@fmease should be good to go, added few more test cases |
@bors r=davidtwco,fmease rollup |
--explain
which exceed our internal limit of 9999
…mease Don't crash on error codes passed to `--explain` which exceed our internal limit of 9999 removed panic in case where we do `--explain > 9999` and added check for it now error looks like this instead of ICE ``` $ rustc.exe --explain E10000 error: E10000 is not a valid error code ``` fixes rust-lang#140647 r? `@fmease`
…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#140234 (Separate dataflow analysis and results) - rust-lang#140614 (Correct warning message in restricted visibility) - rust-lang#140671 (Parser: Recover error from named params while parse_path) - rust-lang#140700 (Don't crash on error codes passed to `--explain` which exceed our internal limit of 9999 ) - rust-lang#140706 ([rustdoc] Ensure that temporary doctest folder is correctly removed even if doctests failed) - rust-lang#140734 (Fix regression from rust-lang#140393 for espidf / horizon / nuttx / vita) - rust-lang#140741 (add armv5te-unknown-linux-gnueabi target maintainer) - rust-lang#140745 (run-make-support: set rustc dylib path for cargo wrapper) r? `@ghost` `@rustbot` modify labels: rollup
removed panic in case where we do
--explain > 9999
and added check for itnow error looks like this instead of ICE
fixes #140647
r? @fmease